-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cdm-radial: sigmet fixes #1295
cdm-radial: sigmet fixes #1295
Conversation
Support for all data types instead of just 5 Universal handling for data types instead of each one separately Support for data types spanning two bytes instead of one is experimental at this point as I didn't have such a file to test Most new data types are returned as raw values meaning no normalization like done for total power, reflectivity, velocity, width, differential reflectivity Added comments and small fixes Ultimately all these changes add support for IDEAM Colombia files Addresses the github issue #1292
Code Style... :/
Another fix necessary to support IDEAM Colombia files In this case not all sweeps of num_sweeps have data, so adapting for this and handling it gracefully. Happens for the radars San_Andres and Corozal.
Hi @michaeldiener , thanks for contributing! I've marked this PR as a draft, please set it to ready for review when it's ready! |
Sorry for another commit today, it was a last minute fix. All done now, thank you. |
I'm really sorry, I have encountered another issue. Keeping this as a draft now for a few days to avoid wasting anybody's time. |
Data types with more than 1 byte fully supported A few small fixes
@haileyajohnson The test here seems to fail due to a connection issue when I'm not mistaken. This seems to happen in a module totally unrelated to the one I modified. Is this a known issue? How to proceed? `dap4.test.TestHyrax > test[5: nc4_strings_comp.nc] STANDARD_ERROR dap4.test.TestHyrax > test[5: nc4_strings_comp.nc] FAILED |
@michaeldiener that looks like a flaky test. I'll rerun your tests and if it fails again I'll turn that one off. |
we're taking a look at the flaky tests but feel free to mark this ready when you want us to review it |
@haileyajohnson Thank you! I'm going to wait for a few more days, just to make sure everything keeps running smoothly in my own server environment. |
Fix for 2 byte data types (was able to find an example file finally)
Fix for reading data from ray for only the number of bins available
Found an example of a file I was looking for eventually. All working well now IMHO. |
Support for all data types instead of just 5 Universal handling for data types instead of each one separately Support for data types spanning two bytes instead of one is experimental at this point as I didn't have such a file to test Most new data types are returned as raw values meaning no normalization like done for total power, reflectivity, velocity, width, differential reflectivity Added comments and small fixes Ultimately all these changes add support for IDEAM Colombia files Addresses the github issue #1292
Code Style... :/
Another fix necessary to support IDEAM Colombia files In this case not all sweeps of num_sweeps have data, so adapting for this and handling it gracefully. Happens for the radars San_Andres and Corozal.
Data types with more than 1 byte fully supported A few small fixes
Fix for 2 byte data types (was able to find an example file finally)
Fix for reading data from ray for only the number of bins available
@michaeldiener I pushed a few changes to your branch and I'm gonna run it against our full test suite, but if that passes I'd say it's ready to go. Thanks again for contributing! |
Please let me go through your changes before merging. At first sight there are a few things I want to have a closer look. |
Fixes for change "PR feedback and cleanup" b89cf77
@haileyajohnson Thanks for the review! |
Thanks! We're having some technical difficulties with our test data at the moment, so I'm waiting on that to resolve to run our tests :) |
Ultimately all these changes add support for IDEAM Colombia files
Addresses the github issue #1292
I worked with this version of the Sigmet/IRIS specification: IRIS-Programming-Guide-M211318EN.pdf
(not attaching it because of copyright)
PR Checklist
until ready for review